Rename conformance tests policy validate -> custom by AlexeyKrasnoperov · Pull Request #6356 · ChainSafe/forest

AlexeyKrasnoperov · 2025-12-16T12:04:21Z

Summary of changes

The conformance tests policy name "validate" is confusing. The name "custom" fits it more, and is self-explanatory.

Changes introduced in this pull request:

Rename conformance tests policy from "validate" to "custom".

Change checklist

I have performed a self-review of my own code,
I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
I have added tests that prove my fix is effective or that my feature works (if possible),
I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

Refactor
- Internal test utility methods were refactored for improved clarity and consistency.

Note: This release contains no user-facing changes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-16T12:04:32Z

Walkthrough

This change renames test helper methods in RpcTest from validate/validate_raw to custom/custom_raw to better reflect their purpose. All internal wiring and test suites are updated to use the new method names consistently.

Changes

Cohort / File(s)	Summary
RpcTest method renaming `src/tool/subcommands/api_cmd/api_compare_tests.rs`	Renamed public test helper methods: `validate` → `custom`, `validate_raw` → `custom_raw`. Updated `identity()` to delegate to `Self::custom()`. Updated internal method calls and documentation. Updated all test suites (`common_tests`, `eth_tests`, `gas_tests_with_tipset`, `validate_message_lookup`, `validate_tagged_tipset_v2`) to use renamed methods.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Verify all validate and validate_raw usages have been replaced with custom and custom_raw respectively
Confirm internal delegation chains (e.g., identity() → Self::custom()) are correct
Check that test suite updates are consistent and no call sites were missed

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: renaming a public test helper method from 'validate' to 'custom' in conformance tests.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch ak/rename-conformance-tests-policy-validate-to-custom

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e2fc9d2 and 881cd50.

📒 Files selected for processing (1)

src/tool/subcommands/api_cmd/api_compare_tests.rs (7 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-09-11T16:03:14.328Z

Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.

Applied to files:

src/tool/subcommands/api_cmd/api_compare_tests.rs

📚 Learning: 2025-08-28T12:52:46.927Z

Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.

Applied to files:

src/tool/subcommands/api_cmd/api_compare_tests.rs

📚 Learning: 2025-08-04T13:36:22.993Z

Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5835
File: src/chain/tests.rs:58-76
Timestamp: 2025-08-04T13:36:22.993Z
Learning: In the Forest codebase, `Vec<u8>` can be used as an `AsyncWrite` implementation in test contexts. The user confirmed that tests using `&mut Vec<u8>` with `export` and `export_v2` functions compile and pass both locally and on CI.

Applied to files:

src/tool/subcommands/api_cmd/api_compare_tests.rs

🧬 Code graph analysis (1)

src/tool/subcommands/api_cmd/api_compare_tests.rs (6)

src/rpc/methods/chain.rs (1)

from_lotus_json (1494-1499)

src/lotus_json/signed_message.rs (1)

from_lotus_json (81-88)

src/rpc/methods/eth.rs (1)

from_lotus_json (651-674)

src/blocks/tipset.rs (1)

from_lotus_json (666-668)

src/lotus_json/mod.rs (5)

from_lotus_json (149-149)

from_lotus_json (573-575)

from_lotus_json (587-592)

from_lotus_json (608-614)

from_lotus_json (633-640)

src/rpc/registry/methods_reg.rs (1)

from_lotus_json (184-186)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

GitHub Check: Build MacOS
GitHub Check: Coverage
GitHub Check: All lint checks
GitHub Check: Build Ubuntu
GitHub Check: cargo-publish-dry-run
GitHub Check: Build forest binaries on Linux AMD64
GitHub Check: tests
GitHub Check: tests-release

🔇 Additional comments (5)

src/tool/subcommands/api_cmd/api_compare_tests.rs (5)

322-365: LGTM! Clean rename from validate to custom.

The method rename improves clarity—"custom" better conveys that these methods accept custom validation logic. The implementation remains unchanged, and the documentation is appropriate for the new name.

368-370: LGTM! Correct delegation to renamed method.

The identity method now correctly delegates to Self::custom instead of the old Self::validate method name.

457-459: LGTM! Test functions updated to use renamed method.

All test functions that previously called RpcTest::validate now correctly use RpcTest::custom. The validation logic in the closures remains unchanged.

Also applies to: 1384-1387, 2182-2214

2589-2596: LGTM! Helper functions updated consistently.

The helper functions validate_message_lookup and validate_tagged_tipset_v2 now correctly use RpcTest::custom. Note that the function names themselves still contain "validate", which is fine—they describe what they validate, not the policy name.

Also applies to: 2598-2610

322-322: Clean refactoring with consistent updates across the codebase.

This PR successfully renames the test policy from "validate" to "custom" throughout the file. All method definitions and call sites are updated consistently with no logic changes. The new name better reflects that these methods accept custom validation logic, aligning with the PR's goal of improving clarity.

Also applies to: 331-331, 369-369, 457-457, 1384-1384, 2182-2182, 2590-2590, 2599-2599

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-12-16T12:18:53Z

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.40%. Comparing base (e2fc9d2) to head (881cd50).

Files with missing lines	Patch %	Lines
src/tool/subcommands/api_cmd/api_compare_tests.rs	0.00%	9 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/tool/subcommands/api_cmd/api_compare_tests.rs	`0.00% <0.00%> (ø)`

... and 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2fc9d2...881cd50. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

LesnyRumcajs

As discussed, my recommendation is to have a change across the board, i.e., basic -> validate_basic, validate -> validate_custom and identity to validate_identity which are a bit more expressive IMO.

Rename conformance tests policy validate -> custom

881cd50

AlexeyKrasnoperov requested a review from a team as a code owner December 16, 2025 12:04

AlexeyKrasnoperov requested review from LesnyRumcajs and hanabi1224 and removed request for a team December 16, 2025 12:04

LesnyRumcajs reviewed Dec 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename conformance tests policy validate -> custom#6356

Rename conformance tests policy validate -> custom#6356
AlexeyKrasnoperov wants to merge 1 commit intomainfrom
ak/rename-conformance-tests-policy-validate-to-custom

AlexeyKrasnoperov commented Dec 16, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 16, 2025 •

edited

Loading

Uh oh!

codecov bot commented Dec 16, 2025

Uh oh!

LesnyRumcajs left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

AlexeyKrasnoperov commented Dec 16, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Other information and links

Change checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Dec 16, 2025

Codecov Report

Uh oh!

LesnyRumcajs left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AlexeyKrasnoperov commented Dec 16, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 16, 2025 •

edited

Loading